update turf #91
update turf #91
Conversation
michaelkirk
force-pushed
the
mkirk/update-turf
branch
from
8241afe to
f4f3cb4
Compare
| guard let slicedLineBehind = LineString(coordinates.reversed()).sliced(from: closest.coordinate, to: coordinates.reversed().last) else { return nil } | ||
| guard let slicedLineInFront = nearByPolyline.sliced(from: closest.coordinate, to: coordinates.last) else { return nil } |
There was a problem hiding this comment.
Why was returning nil here not needed before?
There was a problem hiding this comment.
A good question!
The proximate answer is that the sliced method signature changed. Previously, if you had a LineString with 0 coordinates, if you tried to slice it, it would return another LineString with 0 coordinates. Now it returns nil.
public func sliced(from start: CLLocationCoordinate2D? = nil, to end: CLLocationCoordinate2D? = nil) -> LineString {
// Ported from https://github.com/Turfjs/turf/blob/142e137ce0c758e2825a260ab32b24db0aa19439/packages/turf-line-slice/index.js
guard !coordinates.isEmpty else {
return LineString([])
}
....
public func sliced(from start: LocationCoordinate2D? = nil, to end: LocationCoordinate2D? = nil) -> LineString? {
// Ported from https://github.com/Turfjs/turf/blob/142e137ce0c758e2825a260ab32b24db0aa19439/packages/turf-line-slice/index.js
guard !coordinates.isEmpty else { return nil }Seems a little arbitrary, but whatever. 🤷
In any case, could this conceivably affect behavior?
Well, a few lines below your comment is another line:
guard let pointBehind = slicedLineBehind.coordinateFromStart(distance: userDistanceBuffer) else { return nil }This method in turn ultimately calls:
public func indexedCoordinateFromStart(distance: CLLocationDistance) -> IndexedCoordinate? {
guard let firstCoordinate = coordinates.first else {
return nil
}
...
}This is true for both the old and the new version of turf.
So to summarize, the difference only applies in the degenerate case of getting an empty LineString as input. And in that case, the only difference is we'd now return nil a few lines earlier than we would have returned nil before, but we'll still ultimately return nil.
Bottom line: I don't think there could be an observable change in behavior from this.
| @@ -1071,7 +1074,9 @@ open class NavigationMapView: MLNMapView, UIGestureRecognizerDelegate { | |||
| */ | |||
| @objc public func setOverheadCameraView(from userLocation: CLLocationCoordinate2D, along coordinates: [CLLocationCoordinate2D], for bounds: UIEdgeInsets) { | |||
| self.isAnimatingToOverheadMode = true | |||
| let slicedLine = Polyline(coordinates).sliced(from: userLocation).coordinates | |||
| guard let slicedLine = LineString(coordinates).sliced(from: userLocation)?.coordinates else { | |||
There was a problem hiding this comment.
In which cases does LineString.sliced return nil in this case?
There was a problem hiding this comment.
Same cause as my previous comment - sliced will now return nil rather than an empty LineString in the case that coordinates is empty.
The implications of this one are trickier though — it's possible that this might result in some change in behavior. My wager is that it's OK, since passing empty coordinates seems ill formed in the first place.
And in that case, our response is a no-op, rather than exploding or something scary.
I've just now added a debug assert to build confidence that we're not inadvertently hitting that code path.
What do you think?
|
Looking fine, just have a few questions so that I fully understand the PR. Please resolve the merge conflict and mark as ready for review. |
Previously the IndexedCoordinate returned from `closestPoint` had its `distance` field set to the distance from the closest point on the Line. But it was always intended to be distance from the `start` of the line. Upstream fixed this in: https://github.com/mapbox/turf-swift/pull/107/files So this commit adapts to this new behavior.
michaelkirk
force-pushed
the
mkirk/update-turf
branch
from
f4f3cb4 to
63d8fff
Compare
michaelkirk
force-pushed
the
mkirk/update-turf
branch
from
63d8fff to
b1eb01c
Compare
| guard let slicedLineBehind = LineString(coordinates.reversed()).sliced(from: closest.coordinate, to: coordinates.reversed().last) else { return nil } | ||
| guard let slicedLineInFront = nearByPolyline.sliced(from: closest.coordinate, to: coordinates.last) else { return nil } |
There was a problem hiding this comment.
A good question!
The proximate answer is that the sliced method signature changed. Previously, if you had a LineString with 0 coordinates, if you tried to slice it, it would return another LineString with 0 coordinates. Now it returns nil.
public func sliced(from start: CLLocationCoordinate2D? = nil, to end: CLLocationCoordinate2D? = nil) -> LineString {
// Ported from https://github.com/Turfjs/turf/blob/142e137ce0c758e2825a260ab32b24db0aa19439/packages/turf-line-slice/index.js
guard !coordinates.isEmpty else {
return LineString([])
}
....
public func sliced(from start: LocationCoordinate2D? = nil, to end: LocationCoordinate2D? = nil) -> LineString? {
// Ported from https://github.com/Turfjs/turf/blob/142e137ce0c758e2825a260ab32b24db0aa19439/packages/turf-line-slice/index.js
guard !coordinates.isEmpty else { return nil }Seems a little arbitrary, but whatever. 🤷
In any case, could this conceivably affect behavior?
Well, a few lines below your comment is another line:
guard let pointBehind = slicedLineBehind.coordinateFromStart(distance: userDistanceBuffer) else { return nil }This method in turn ultimately calls:
public func indexedCoordinateFromStart(distance: CLLocationDistance) -> IndexedCoordinate? {
guard let firstCoordinate = coordinates.first else {
return nil
}
...
}This is true for both the old and the new version of turf.
So to summarize, the difference only applies in the degenerate case of getting an empty LineString as input. And in that case, the only difference is we'd now return nil a few lines earlier than we would have returned nil before, but we'll still ultimately return nil.
Bottom line: I don't think there could be an observable change in behavior from this.
| @@ -1071,7 +1074,9 @@ open class NavigationMapView: MLNMapView, UIGestureRecognizerDelegate { | |||
| */ | |||
| @objc public func setOverheadCameraView(from userLocation: CLLocationCoordinate2D, along coordinates: [CLLocationCoordinate2D], for bounds: UIEdgeInsets) { | |||
| self.isAnimatingToOverheadMode = true | |||
| let slicedLine = Polyline(coordinates).sliced(from: userLocation).coordinates | |||
| guard let slicedLine = LineString(coordinates).sliced(from: userLocation)?.coordinates else { | |||
There was a problem hiding this comment.
Same cause as my previous comment - sliced will now return nil rather than an empty LineString in the case that coordinates is empty.
The implications of this one are trickier though — it's possible that this might result in some change in behavior. My wager is that it's OK, since passing empty coordinates seems ill formed in the first place.
And in that case, our response is a no-op, rather than exploding or something scary.
I've just now added a debug assert to build confidence that we're not inadvertently hitting that code path.
What do you think?
This is a draft because it is based on #90, so please review that first.
Description
Taking a smaller bite out of #89, this PR only updates our turf dependency.